• Things that didn't have to be or shouldn't be in that way # 1

    Author: theany

    12.02

    17/02/2025

  • I'm translating a JS code base into Java and I wanted to share things I found stupid/weird in the earlier version of the code base. There're a lot of them.

    const getUser = await ChannelUserModel.find({
                channel_id: channel_id,
                user_id: user
            });
            if (getUser) {
                await ChannelUserModel.deleteOne({
                    channel_id: channel_id,
                    user_id: user
                });
                await leaveChannelByRoleModel.create({
                    channel_id: channel_id,
                    action_by: user,
                    subserver_id: subserver_id,
                    user_id: user
                });
            } else {
                await leaveChannelByRoleModel.create({
                    channel_id: channel_id,
                    action_by: user,
                    subserver_id: subserver_id,
                    user_id: user
                });
            }

    The above block handles a user leaving a channel. So the problem here is that the else statement is completely unnecessary. They could've get rid of the else block and carry the other copy of the same block outside of if. It'll execute no matter what in this case. It's just repetitive.

    Also another problem is they're doing checks with find. It'll return an empty list if no matches but it'll return a list non the less. So the issue that will cause is getUser is will always evaluate to true. That causes the else block to never work. So another redundancy.

    And the last problem is that find is unnecessary. deleteOne will do nothing if there's no match. This code also does nothing different than original path when there's no match. So it's just causing an unnecessary branching.